Conversation
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Co-authored-by: David Uzunov <DavidUzunov@users.noreply.github.com> Co-authored-by: Bailey Say <baileyasay@gmail.com>
…Firmware into GrandCanMigration
Co-authored-by: Bailey Say <baileyasay@gmail.com> Co-authored-by: David Uzunov <DavidUzunov@users.noreply.github.com>
… into GrandCanMigration
|
Dash Panel good to go |
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
# ECU CAN Migration ## Problem and Scope Part of the Grand CAN Migration ## Description CAN Migration ## Gotchas and Limitations ## Testing - [x] HOOTL testing - [ ] HITL testing - [ ] Human tested ### Testing Details ctest ## Larger Impact Must-do. ## Additional Context and Ticket #390 --------- Co-authored-by: kzwicker <kzwicker@users.noreply.github.com> Co-authored-by: khoulihan27 <khoulihan27@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
ECU/Core/Src/main.c
Outdated
| if (getRTT(CCU) != PINGTIMEOUT_VALUE) { | ||
| // halt if CCU is connected | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
Under no scenario can the ECU just throw up it's hands and declare failure, this should be a very clearly logged event (debug message and logomatic) but not a half. (Also, this is not a valid halt, see Error_Handler() which is).
The CCU will never (electrically / mechanically) be connected to the car, this is a footgun waiting to happen if some node accidentally uses the 0x02 identifier or a message ID gets corrupted into it
There was a problem hiding this comment.
Also, you no longer send to this node anymore so this check does nothing
There was a problem hiding this comment.
ECU/Application/Src/Pinging.c
Outdated
| #define PING_LIST(OP) \ | ||
| OP(GR_BCU, 0) \ | ||
| OP(GR_DASH_PANEL, 1) \ | ||
| OP(GR_CCU, 2) | ||
| OP(BCU, 0) \ | ||
| OP(Dash_Panel, 1) |
There was a problem hiding this comment.
This list is not up to date with the current specification
- BCU Node Status
- GR Inverter Status
- Fan Controller 1 Status
- Fan Controller 2 Status
- Fan Controller 3 Status
- Dash Panel Status
- TCM Node Status
- Reserved
- (Another byte likely added later for sensors when those are available, but for now do not worry about them)
There was a problem hiding this comment.
ECU/Application/Src/Pinging.c
Outdated
| ECU_CAN_Send(GRCAN_BUS_PRIMARY, IDsToBePinged[i], MSG_PING, &(GRCAN_PING_MSG){timestamp}, sizeof(GRCAN_PING_MSG)); | ||
| } |
There was a problem hiding this comment.
Please implement something to not send CAN messages to 8-16 nodes at once, your buffer is only 10 messages
There was a problem hiding this comment.
| pingMsg.tx_header.TxFrameType = FDCAN_DATA_FRAME; | ||
| pingMsg.tx_header.ErrorStateIndicator = FDCAN_ESI_ACTIVE; | ||
| pingMsg.tx_header.DataLength = sizeof(GR_OLD_PING_MSG); | ||
| pingMsg.tx_header.DataLength = sizeof(GRCAN_PING_MSG); |
There was a problem hiding this comment.
This is not correct, see https://discord.com/channels/756738476887638107/1457989912069537906/1489483629427163216
There was a problem hiding this comment.
| sendECUMsg.tx_header.TxFrameType = FDCAN_DATA_FRAME; | ||
| sendECUMsg.tx_header.ErrorStateIndicator = FDCAN_ESI_ACTIVE; | ||
| sendECUMsg.tx_header.DataLength = sizeof(GR_OLD_DASH_STATUS_MSG); | ||
| sendECUMsg.tx_header.DataLength = sizeof(GRCAN_DASH_STATUS_MSG); |
There was a problem hiding this comment.
This is not correct, see https://discord.com/channels/756738476887638107/1457989912069537906/1489483629427163216
There was a problem hiding this comment.
| GRCAN_MSG_ID msg_id = GETBITS(ID, 12, 12); | ||
| GRCAN_NODE_ID node_id = GETBITS(ID, 4, 8); |
There was a problem hiding this comment.
GETBITS and related is only intended for CAN message bit flags (eg button and LED bit flags) and can be changed to follow MSB/LSB ordering. These bits are fixed and you should use manual bit manipulations to extract the values.
| GRCAN_MSG_ID msg_id = GETBITS(ID, 12, 12); | |
| GRCAN_NODE_ID node_id = GETBITS(ID, 4, 8); | |
| GRCAN_MSG_ID msg_id = (0x000FFF00 & ID) >> 8; | |
| GRCAN_NODE_ID node_id = (0x0FF00000 & ID) >> 20; |
There was a problem hiding this comment.
| void BMSLights(ECU_StateData *stateLump) | ||
| { | ||
| bool light = 0; | ||
| light |= stateLump->max_cell_temp_c > CRITICAL_MAX_CELL_TEMP_C; | ||
| light |= stateLump->ts_voltage > CRITICAL_TS_VOLTAGE; | ||
| light |= bmsFailure(stateLump); | ||
| // TODO: interrupted/missing BMS vals | ||
| GR_OLD_DASH_CONFIG_MSG message = {.led_bits = light}; | ||
| ECU_CAN_Send(GR_OLD_BUS_PRIMARY, GR_DASH_PANEL, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| GRCAN_DASH_CONFIG_MSG message = {.led_bits = light}; | ||
| ECU_CAN_Send(GRCAN_BUS_PRIMARY, GRCAN_Dash_Panel, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| } | ||
|
|
||
| void IMDLights(ECU_StateData *stateLump) | ||
| { | ||
| uint8_t light = 0; | ||
| // TODO: isolation failure? | ||
| light |= imdFailure(stateLump); | ||
| GR_OLD_DASH_CONFIG_MSG message = {.led_bits = (light << 1)}; | ||
| ECU_CAN_Send(GR_OLD_BUS_PRIMARY, GR_DASH_PANEL, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| GRCAN_DASH_CONFIG_MSG message = {.led_bits = (light << 1)}; | ||
| ECU_CAN_Send(GRCAN_BUS_PRIMARY, GRCAN_Dash_Panel, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| } | ||
|
|
||
| void BSPDLights(ECU_StateData *stateLump) | ||
| { | ||
| uint8_t light = 0; | ||
| // TODO: isolation failure? | ||
| light |= bspdFailure(stateLump); | ||
| GR_OLD_DASH_CONFIG_MSG message = {.led_bits = (light << 2)}; | ||
| ECU_CAN_Send(GR_OLD_BUS_PRIMARY, GR_DASH_PANEL, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| GRCAN_DASH_CONFIG_MSG message = {.led_bits = (light << 2)}; | ||
| ECU_CAN_Send(GRCAN_BUS_PRIMARY, GRCAN_Dash_Panel, MSG_DASH_CONFIG, &message, sizeof(message)); | ||
| } |
There was a problem hiding this comment.
Please use SetBitInByte from the BitManipulations library as it will correctly handle LSB/MSB transitions and such
There was a problem hiding this comment.
Signed-off-by: Daniel Hansen <dchansen06@gmail.com>
| break; | ||
| } | ||
| respondToPing(sender_id, ((GR_OLD_PING_MSG *)data)->timestamp); | ||
| respondToPing(sender_id, ((GRCAN_PING_MSG *)data)->timestamp); |
There was a problem hiding this comment.
Please note that the debugger may attempt to ping you and will want a valid response (so you would need to respond to the ping as a client or server depending on the node ID)
There was a problem hiding this comment.
Initial CAN Specification Migration
Problem and Scope
Migrate away from old CAN specification to new (auto-generated) one
Transitioning from
GR_OLD_CAN_MESSAGEStoCANfiguratorinterface link libraryDescription
Each group will work on migrating their own board on this PR to avoid merge conflicts (separate from other boards), this can be done by having a VS Code Live Share on this branch and only fixing code in their scope and syncing often
Gotchas and Limitations
Subject to change, this is first pass
Testing
Testing Details
Compilation and groups checking their projects
Larger Impact
Migrating fully to URCA and new CAN implementation
Additional Context and Ticket
Resolves #261